Skip to content

(fix) avoid automatic key offset after track load with keylock mode 'Keep current key'#16460

Merged
daschuer merged 3 commits into
mixxxdj:2.6from
ronso0:keyoffset-fix
May 25, 2026
Merged

(fix) avoid automatic key offset after track load with keylock mode 'Keep current key'#16460
daschuer merged 3 commits into
mixxxdj:2.6from
ronso0:keyoffset-fix

Conversation

@ronso0
Copy link
Copy Markdown
Member

@ronso0 ronso0 commented May 14, 2026

Final fix for #12027

The issue is that track load triggers a cascade of calls in KeyControl which lead to unintended und annoying key offsets on track load with

  • keylock ON, rate off center
  • keylock mode "Keep current key"
  • Reset Pitch on track load option enabled

Reproduce issue / confirm fix:

  1. rate centered, pitch = 0, pitch_adjust = 0, keylock disabled
  2. load a track
  3. move rate slider off center -> key is off
  4. load a new track
  5. enable keylock
  6. center rate slider
  7. reset key with reset_key control -> key is reset, pitch = 0
  8. load another track
    -> previously: pitch != 0
    -> now: pitch = 0

The issue stems from the fact that in #1222 we decided to not make the Key knob (pitch_adjust) jump when locking so that users can easily turn back to original key with it.
The consequence is that pitch_adjust should stay steady on track load.
The culprit is that BaseTrackPlayerImpl::slotTrackLoaded() did always reset pitch_adjust to 0 on track load.

The fix is to reset pitch (not pitch_adjust) to 0 in this case (key locked with "Keep current key").

Works beautifully and key tests pass 🤷‍♂️


FWIW actually we should refactor KeyControl as it seems too complicated with all its interleaved, potentially infinitly looping valueChanged() connections, as well as the interaction with BaseTrackPlayer and EngineBuffer, but I'm certainly not gonna refactor that, so consider this a quick but solid fix.

@ronso0 ronso0 changed the base branch from main to 2.6 May 14, 2026 22:20
@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented May 15, 2026

Yes, this definitly fixes it for me. No key issues anymore in a 5h set 🎉

Will fixup the unused param issues and add a regression test soon.

Copy link
Copy Markdown
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Thank you.

@daschuer daschuer merged commit 347a44b into mixxxdj:2.6 May 25, 2026
16 checks passed
@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented May 25, 2026

Great, thanks for you review!
After years of occasional debugging it finally just works : )

@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented May 25, 2026

will take care of the conflicts with main.

@ronso0 ronso0 mentioned this pull request May 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants